packages/dashmate/src/commands/ssl/cleanup.js (1)

6-10: Enhance command description with account limits and impact.

Consider adding more context to help users understand the importance and impact of this command.

 static description = `Cleanup Zero SSL certificate

-Cancel all drafted or pending validation certificates on ZeroSSL
+Cancel all drafted or pending validation certificates on ZeroSSL.
+
+This command helps prevent reaching ZeroSSL certificate limits:
+- Free accounts: 3 certificates
+- Paid accounts: 10,000 certificates
+
+Note: Only draft and pending validation certificates will be removed.
`;

packages/dashmate/src/listr/tasks/ssl/zerossl/cleanupZeroSSLCertificatesTaskFactory.js (1)

72-72: Enhance progress output for clarity

Make the progress output more descriptive to improve the user experience.

Apply this diff to update the progress message:

-            observer.next(chalk`{green ${ctx.canceled}} / {red ${ctx.errored}} / ${ctx.total}`);
+            observer.next(chalk`Canceled: {green ${ctx.canceled}}, Failed: {red ${ctx.errored}}, Total: ${ctx.total}`);

Configuration used: CodeRabbit UI
Review profile: CHILL

- export default async function listCertificates(apiKey, statuses = [], page = 1, search = undefined) {
-   let url = `https://api.zerossl.com/certificates?access_key=${apiKey}&limit=1000&page=${page}`;
+ const DEFAULT_PAGE_SIZE = 1000;
+ 
+ export default async function listCertificates(apiKey, statuses = [], page = 1, search = undefined, pageSize = DEFAULT_PAGE_SIZE) {
+   if (!Number.isInteger(page) || page < 1) {
+     throw new Error('Page must be a positive integer');
+   }
+   
+   let url = `https://api.zerossl.com/certificates?access_key=${apiKey}&limit=${pageSize}&page=${page}`;

#!/bin/bash
# Description: Verify that the cleanup command properly imports and uses these dependencies

# Test 1: Check if cleanup command imports these dependencies
rg -l "cleanupZeroSSLCertificatesTask|cancelCertificate" "packages/dashmate/src/commands/"

# Test 2: Check the cleanup command implementation
rg -A 10 "class.*Cleanup.*Command" "packages/dashmate/src/commands/"

#!/bin/bash
# Check the complete implementation of cleanup command
cat packages/dashmate/src/commands/ssl/cleanup.js

# Check the implementation of cleanupZeroSSLCertificatesTaskFactory
cat packages/dashmate/src/listr/tasks/ssl/zerossl/cleanupZeroSSLCertificatesTaskFactory.js

-                task.output = 'Found ${ctx.certificates.length} certificates';
+                task.output = `Found ${ctx.certificates.length} certificates`;

-              task.title = 'Cancel ${ctx.certificates.length} certificates';
+              task.title = `Cancel ${ctx.certificates.length} certificates`;

packages/dashmate/src/helper/scheduleRenewZeroSslCertificateFactory.js (1)

Line range hint 62-64: Consider implementing the suggested dynamic configuration update.

The TODO comment suggests using Envoy's dynamic filesystem configuration, which would be a valuable improvement. This could eliminate the need for Gateway restarts when certificates are updated.

Would you like me to create a GitHub issue to track the implementation of dynamic configuration updates using Envoy's filesystem-based configuration?

packages/dashmate/src/listr/tasks/ssl/zerossl/cleanupZeroSSLCertificatesTaskFactory.js (2)

86-94: Consider adding more detailed renderer options for better progress tracking

The current renderer configuration could be enhanced to provide better visibility of the cleanup progress.

Apply this diff to add more renderer options:

         options: {
           persistentOutput: true,
+          bottomBar: true,
         },
       },
     ], {
       rendererOptions: {
         showErrorMessage: true,
+        collapseErrors: false,
+        collapseSubtasks: false,
       },
     });

---

20-21: Add a warning message before certificate cleanup

Since this is a destructive operation that cancels certificates, it would be helpful to add a warning message.

Add a new task at the beginning of the Listr tasks:

   function cleanupZeroSSLCertificatesTask(config) {
     const apiKey = config.get('platform.gateway.ssl.providerConfigs.zerossl.apiKey', true);
 
     return new Listr([
+      {
+        title: 'Warning',
+        task: (_, task) => {
+          task.output = chalk.yellow(
+            'This operation will cancel all draft and pending validation certificates.\n' +
+            'This action cannot be undone.'
+          );
+        },
+        options: { persistentOutput: true },
+      },
       {
         title: 'Collect drafted and pending validation certificates',

packages/dashmate/src/listr/tasks/ssl/zerossl/obtainZeroSSLCertificateTaskFactory.js (3)

Line range hint 123-134: Add cleanup of draft certificates before creating new ones.

To prevent accumulation of draft certificates (as mentioned in PR #2297), consider adding a cleanup step before creating a new certificate. This would help prevent hitting account limits, especially for free accounts limited to three certificates.

      {
        title: 'Create a certificate',
        skip: (ctx) => ctx.certificate,
        task: async (ctx) => {
+         // Clean up any existing draft certificates before creating a new one
+         const certificates = await listCertificates(ctx.apiKey);
+         const draftCerts = certificates.filter(cert => 
+           ['draft', 'pending_validation'].includes(cert.status));
+         for (const cert of draftCerts) {
+           await cancelCertificate(cert.id, ctx.apiKey);
+         }
+
          ctx.certificate = await createZeroSSLCertificate(
            ctx.csr,
            ctx.externalIp,
            ctx.apiKey,

---

Line range hint 190-217: Consider making retry parameters configurable.

The certificate download retry mechanism uses hard-coded values:

• 50 retry attempts
• 5-second wait between attempts
• Total potential wait time: 250 seconds

Consider making these parameters configurable to allow users to adjust based on their needs and network conditions.

+     const DEFAULT_DOWNLOAD_RETRY_COUNT = 50;
+     const DEFAULT_DOWNLOAD_RETRY_DELAY = 5000;
      {
        title: 'Download certificate file',
        skip: (ctx) => ctx.isBundleFilePresent,
        task: async (ctx, task) => {
-         for (let retry = 0; retry <= 50; retry += 1) {
-           await wait(5000);
+         const retryCount = ctx.downloadRetryCount || DEFAULT_DOWNLOAD_RETRY_COUNT;
+         const retryDelay = ctx.downloadRetryDelay || DEFAULT_DOWNLOAD_RETRY_DELAY;
+         for (let retry = 0; retry <= retryCount; retry += 1) {
+           await wait(retryDelay);

---

Line range hint 218-249: Set secure file permissions for sensitive files.

The code writes sensitive cryptographic materials (private key, CSR, certificate) without explicitly setting secure file permissions. This could potentially expose these files if default permissions are too permissive.

      {
        title: 'Save certificate private key file',
        enabled: (ctx) => !ctx.isPrivateKeyFilePresent,
        task: async (ctx, task) => {
-         fs.writeFileSync(ctx.privateKeyFilePath, ctx.privateKeyFile, 'utf8');
+         fs.writeFileSync(ctx.privateKeyFilePath, ctx.privateKeyFile, {
+           encoding: 'utf8',
+           mode: 0o600  // Read/write for owner only
+         });

          // eslint-disable-next-line no-param-reassign
          task.output = ctx.privateKeyFilePath;
        },
      },
      {
        title: 'Save certificate request file',
        enabled: (ctx) => !ctx.isCsrFilePresent,
        task: async (ctx, task) => {
-         fs.writeFileSync(ctx.csrFilePath, ctx.csr, 'utf8');
+         fs.writeFileSync(ctx.csrFilePath, ctx.csr, {
+           encoding: 'utf8',
+           mode: 0o644  // World-readable but only owner-writable
+         });

          // eslint-disable-next-line no-param-reassign
          task.output = ctx.csrFilePath;
        },
      },

Configuration used: CodeRabbit UI
Review profile: CHILL

Learnt from: shumkov
PR: dashpay/platform#2298
File: packages/dashmate/src/listr/tasks/ssl/zerossl/cleanupZeroSSLCertificatesTaskFactory.js:77-81
Timestamp: 2024-10-31T15:44:50.617Z
Learning: In the `cleanupZeroSSLCertificatesTask`, if some certificates are not canceled, we want to abort listr, as it's the last task.

Learnt from: shumkov
PR: dashpay/platform#2298
File: packages/dashmate/src/listr/tasks/ssl/zerossl/cleanupZeroSSLCertificatesTaskFactory.js:66-69
Timestamp: 2024-10-31T15:43:16.094Z
Learning: In Listr tasks, when in debug mode, prefer using `console.warn` for logging instead of `task.output`.

Learnt from: shumkov
PR: dashpay/platform#2298
File: packages/dashmate/src/commands/ssl/cleanup.js:33-50
Timestamp: 2024-10-31T15:37:28.986Z
Learning: In `packages/dashmate/src/commands/ssl/cleanup.js`, the `cleanupZeroSSLCertificatesTask` function already includes progress reporting within its nested `Listr` tasks, so additional progress reporting in the `CleanupCommand` is unnecessary.

Learnt from: shumkov
PR: dashpay/platform#2297
File: packages/dashmate/src/listr/tasks/ssl/zerossl/obtainZeroSSLCertificateTaskFactory.js:148-150
Timestamp: 2024-10-31T14:41:42.158Z
Learning: In the `obtainZeroSSLCertificateTaskFactory` function in `packages/dashmate/src/listr/tasks/ssl/zerossl/obtainZeroSSLCertificateTaskFactory.js`, the application already fails appropriately if `configFileRepository.write(configFile)` fails, so additional error handling around this operation is unnecessary.

#!/bin/bash
# Description: Verify the implementation of obtainZeroSSLCertificateTask
# to ensure it properly handles async operations after the refactor

# Search for the task factory implementation
ast-grep --pattern 'obtainZeroSSLCertificateTask = $$$'

# Look for any Listr usage in the implementation
rg -A 5 'new Listr' packages/dashmate/src/ssl

#!/bin/bash
# Let's try a different approach to find the implementation

# Search for the file containing obtainZeroSSLCertificateTask
rg -l "obtainZeroSSLCertificateTask" packages/dashmate/src

# Search for the implementation with more context
rg -B 2 -A 10 "obtainZeroSSLCertificateTask" packages/dashmate/src

# Look for Listr usage in SSL-related files
fd . packages/dashmate/src/ssl --type f --exec rg -l "Listr" {}

#!/bin/bash
# Let's examine the implementation of obtainZeroSSLCertificateTaskFactory to verify async handling

# Get the full content of the task factory implementation
cat packages/dashmate/src/listr/tasks/ssl/zerossl/obtainZeroSSLCertificateTaskFactory.js

#!/bin/bash
# Search for similar patterns in other task factories to confirm consistency
ast-grep --pattern 'const tasks = $_($$$);
  await tasks.run($$$);'

#!/bin/bash
# Let's try a broader search for task execution patterns
rg -U "const tasks.*\n.*await.*run" --type js

# Also search for obtainZeroSSLCertificateTask usage
rg "obtainZeroSSLCertificateTask" --type js -A 3

# And check the implementation of obtainZeroSSLCertificateTask
ast-grep --pattern 'const obtainZeroSSLCertificateTask = $_($$$)'

#!/bin/bash
# Let's examine the implementation of obtainZeroSSLCertificateTaskFactory to understand the task structure
cat packages/dashmate/src/listr/tasks/ssl/zerossl/obtainZeroSSLCertificateTaskFactory.js

# Also check the Listr task implementation pattern in other similar files
rg "new Listr\(\[" --type js -A 3